[DeepCompile] Fix KeyError on frozen parameters in ZeRO-3 - #8214
[DeepCompile] Fix KeyError on frozen parameters in ZeRO-3#8214roycho96 wants to merge 2 commits into
Conversation
init_z3 looks up every module parameter in the stage-3 optimizer's
__param_id_to_grad_partition map, but that map is built only from the
trainable parameter groups: _get_trainable_parameter_groups filters out
parameters with requires_grad=False. With ZeRO-3 and
"compile": {"deepcompile": true}, engine.compile() therefore raised
KeyError on the first frozen parameter, which breaks any LoRA/PEFT or
partial-freeze run.
Register frozen parameters with an empty grad buffer instead, mirroring
the requires_grad guard already used by set_grad_buffer in the same file.
The empty buffer is never consumed: add_gather_and_reduce schedules no
reduce op for a parameter without a grad node, and the buffer is only
read from flushReduceBucket via those reduce ops. Frozen parameters are
still registered with the native handle because they are partitioned and
need gather/release ops in the forward graph.
Add TestDeepCompile::test_frozen_params using SimpleFrozenModel. It
compares loss and parameters against a ZeRO-0 eager baseline across the
warmup boundary, so the prefetch and selective-gather passes also run
with frozen parameters present. compare_loss takes an optional model_cls
argument, leaving the existing callers unchanged.
Signed-off-by: Sung Hyun Cho <hope5487@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c38d6d9d14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for p in engine.module.parameters(): | ||
| grad_buffer = torch.Tensor() | ||
| if use_opt: | ||
| # Frozen params (e.g. the base weights of a LoRA setup) are absent from the optimizer's |
There was a problem hiding this comment.
Add the required Signed-off-by trailer
This is a non-merge commit but its message has no Signed-off-by: trailer, which violates the repository's Commit & CI requirement and can cause DCO/signoff validation to reject the change. Please recommit/amend with --signoff using the configured Git identity before merging.
AGENTS.md reference: AGENTS.md:L6-L8
Useful? React with 👍 / 👎.
Under ZeRO-3 with
"compile": {"deepcompile": true},engine.compile()fails with aKeyErrorif the model has any parameter withrequires_grad=False. LoRA/PEFT and partial-freeze runs cannot use DeepCompile at all.init_z3()looks up a grad partition for every module parameter, but the stage-3 optimizer builds that map only for the parameters it owns._get_trainable_parameter_groups()dropsrequires_grad=Falseparams (stage3.pyL651), and the map is filled from the resultingfp16_groups(stage3.pyL706-720), so the first frozen parameter is simply not there.Repro
ds_id 0is the frozen layer's weight. We originally hit this through the HF Trainer path with a LoRA adapter, where the crash happens insideaccelerator.prepare()when accelerate callsengine.compile(). Also reproduces on the released 0.19.2 and 0.19.3.Fix
Skip the lookup for frozen parameters and leave them with the empty buffer that the optimizer-less path (
use_opt == False) already uses for every parameter.Nothing reads that buffer for a frozen parameter.
add_gather_and_reduce()skips parameters whose grad node isNone(passes/zero3_compile.pyL130), and the registered buffer is only consumed byflushReduceBucketthrough those reduce ops (csrc/compile/z3.cppL261, L282, L311).set_grad_buffer()a few lines below already applies the samerequires_gradguard against the same map. Frozen parameters are still registered with the native handle, since they are partitioned and need gather/release ops in the forward graph.Test
TestDeepCompile::test_frozen_paramsruns the existingSimpleFrozenModelfor 10 steps on 2 ranks with ZeRO-3 + deepcompile, comparing loss and parameters against a ZeRO-0 eager baseline. 10 steps so the run crosses theWARMUPboundary and the prefetch and selective-gather passes also see the frozen parameters.compare_loss()takes an optionalmodel_cls, so the existing callers are unchanged.Without the fix the test fails at
engine.compile()with the sameKeyError.Notes
ZeRO-1/2 does not hit this, since
init_z1()iteratesoptimizer.bit16_groups, which contains only trainable parameters. I have not checked whether frozen parameters work end to end there, so this change is scoped to ZeRO-3.grad_partitions.get(p.ds_id, torch.Tensor())would also cover a trainable parameter that was never passed to the optimizer, but that case is already broken in eager ZeRO-3, so I kept the narrower guard to matchset_grad_buffer(). Happy to switch if you prefer the wider one.